Feat/paste pictures into editor #472
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
@caushcani is attempting to deploy a commit to the Listinai Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request involve updates to several components in the frontend application, primarily focusing on enhancing media upload functionality. A new entry is added to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor
participant AddEditModal
participant Uppy
participant Server
User->>Editor: Paste media
Editor->>AddEditModal: Trigger onPaste
AddEditModal->>Uppy: Initiate media upload
Uppy->>Server: Upload media
Server-->>Uppy: Return upload URL
Uppy-->>AddEditModal: Notify upload success
AddEditModal->>Editor: Update media state
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Hi @nevo-david , could you please take a look at this PR when you have time? Thank you! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (5)
apps/frontend/src/components/launches/add.edit.model.tsx (5)
Line range hint
10-10: Avoid using@ts-ignoreby providing TypeScript definitionsInstead of suppressing TypeScript errors with
// @ts-ignore, consider installing type definitions forreact-use-keypressor creating a custom declaration to ensure type safety.
391-391: Specify a more precise type for_fileparameterThe
_fileparameter inuploadMediaToServeris currently typed asany. Specify a more precise type, such asFile, to enhance type safety and code clarity.
396-396: SimplifyallowedFileTypesinitializationInstead of splitting a string, define the
allowedFileTypesarray directly for better readability.- allowedFileTypes: 'image/*,video/mp4'.split(','), + allowedFileTypes: ['image/*', 'video/mp4'],
413-413: Simplify null checks using optional chainingYou can simplify the null checks by using optional chaining:
- if (mediaToAdd && mediaToAdd.path && mediaToAdd.id) { + if (mediaToAdd?.path && mediaToAdd.id) {🧰 Tools
🪛 Biome (1.9.4)
[error] 413-413: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
532-532: Simplify null checks using optional chainingUse optional chaining to simplify the condition:
- if (clipboardData && clipboardData.items) { + if (clipboardData?.items) {🧰 Tools
🪛 Biome (1.9.4)
[error] 532-532: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
.gitignore(1 hunks)apps/frontend/src/components/launches/add.edit.model.tsx(5 hunks)apps/frontend/src/components/launches/editor.tsx(1 hunks)apps/frontend/src/components/media/media.component.tsx(3 hunks)apps/frontend/src/components/media/new.uploader.tsx(2 hunks)apps/frontend/src/components/settings/teams.component.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- apps/frontend/src/components/settings/teams.component.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
apps/frontend/src/components/launches/add.edit.model.tsx
[error] 413-413: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 532-532: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (3)
apps/frontend/src/components/launches/add.edit.model.tsx (1)
397-397: Verify if maxFileSize of 1GB is appropriate
The maxFileSize is set to 1GB (1000000000 bytes). Verify if allowing such large file uploads is appropriate, as it may impact performance and resource usage.
apps/frontend/src/components/launches/editor.tsx (1)
57-57: LGTM!
The addition of the onPaste prop effectively extends the CopilotTextarea to handle paste events.
apps/frontend/src/components/media/media.component.tsx (1)
226-231: Verify the impact of removing currentMedia state
By removing the currentMedia state and relying directly on value, ensure that the component behavior remains correct and there are no unintended side effects.
| @@ -84,9 +85,13 @@ export function MultipartFileUploaderAfter({ | |||
|
|
|||
| uppy2.on('complete', (result) => { | |||
| onUploadSuccess(result); | |||
| console.log('uppy complete:', result) | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove console.log statement from production code
Remove the console.log('uppy complete:', result); statement to prevent unnecessary logging in production.
| console.log('uppy upload success:file', file) | ||
| console.log('uppy upload success:response', response) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove console.log statements from production code
Remove these console.log statements to clean up the code:
- console.log('uppy upload success:file', file)
- console.log('uppy upload success:response', response)| const newMedia = [...(currentMedia || []), m]; | ||
| setCurrentMedia(newMedia); | ||
| const newMedia = [...(value || []), m]; | ||
| console.log('new media inside component', newMedia); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove debugging console.log statement
Remove the console.log('new media inside component', newMedia); used for debugging to keep the code clean.
|
Hi @caushcani I have just tried it, it looks like it sends a request to Uppy but it's not attaching any files. |
|
HI Nevo, here it is. Screen-Recording.webmPayload: |
|
Does it work for you with command / ctrl + v ? |
|
Yes, it works. Can u send me a video how you paste the image and network request? Screen-Recording.1.webm |
|
@nevo-david, I made an addition here. Can you try it now ? |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
apps/frontend/src/components/launches/add.edit.model.tsx (2)
530-557: Simplify clipboard data handlingThe clipboard data handling can be simplified using optional chaining and array methods.
Consider this more concise implementation:
onPaste={(event) => { - const clipboardData = - event.clipboardData || - ( - window as Window & - typeof globalThis & { - clipboardData: DataTransfer; - } - ).clipboardData; - if (clipboardData && clipboardData.items) { - for (const item of Array.from(clipboardData.items)) { - const mediaTypes = ['image', 'video']; - - if (mediaTypes.some((type) => item.type.indexOf(type) !== -1)) { - const media = item.getAsFile(); - if(media){ - uploadMediaToServer(media, index); - } - } - } - } + const clipboardData = event.clipboardData ?? window?.clipboardData; + const mediaTypes = ['image', 'video']; + + Array.from(clipboardData?.items ?? []) + .filter(item => mediaTypes.some(type => item.type.startsWith(type))) + .forEach(item => { + const media = item.getAsFile(); + if (media) { + uploadMediaToServer(media, index); + } + }); }}🧰 Tools
🪛 Biome (1.9.4)
[error] 539-539: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
292-297: Improve readability of character count validationThe nested conditions for character count validation could be simplified.
Consider this more readable approach:
-key.value.some((p) => { - return ( - countCharacters(p.content, key?.integration?.identifier || '') > - (key.maximumCharacters || 1000000) - ); -}) +const maxChars = key.maximumCharacters ?? 1000000; +const identifier = key.integration?.identifier ?? ''; +key.value.some(p => countCharacters(p.content, identifier) > maxChars)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
apps/frontend/src/components/launches/add.edit.model.tsx(5 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/frontend/src/components/launches/add.edit.model.tsx
[error] 413-413: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 539-539: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (1)
apps/frontend/src/components/launches/add.edit.model.tsx (1)
53-55: LGTM: Required imports added correctly
The new imports are properly structured and necessary for the media upload functionality.
| const uploadMediaToServer = (_file: File, index: number) => { | ||
| const uppy2 = new Uppy({ | ||
| autoProceed: true, | ||
| restrictions: { | ||
| maxNumberOfFiles: 1, | ||
| allowedFileTypes: ['image/*', 'video/mp4'], | ||
| maxFileSize: 1000000000, | ||
| }, | ||
| }); | ||
|
|
||
| const { plugin, options } = getUppyUploadPlugin( | ||
| storageProvider, | ||
| fetch, | ||
| backendUrl | ||
| ); | ||
| uppy2.use(plugin, options); | ||
| uppy2.addFile(_file); | ||
|
|
||
| uppy2.on('complete', (result) => { | ||
| if (result) { | ||
| const mediaToAdd = result?.successful![0].response?.body; | ||
|
|
||
| if (mediaToAdd && mediaToAdd.path && mediaToAdd.id) { | ||
| const newMedia: { | ||
| path: string; | ||
| id: string; | ||
| }[] = [ | ||
| ...(value[index].image || []), | ||
| { path: mediaToAdd.path, id: mediaToAdd.id }, | ||
| ]; | ||
| changeImage(index)({ target: { name: 'image', value: newMedia } }); | ||
| } | ||
| } | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling and loading state management
While the upload implementation is functional, it could be improved with:
- Error handling for upload failures
- Loading state management to provide user feedback during uploads
Consider applying these improvements:
const uploadMediaToServer = (_file: File, index: number) => {
+ const [isUploading, setIsUploading] = useState(false);
const uppy2 = new Uppy({
autoProceed: true,
restrictions: {
maxNumberOfFiles: 1,
allowedFileTypes: ['image/*', 'video/mp4'],
maxFileSize: 1000000000,
},
});
const { plugin, options } = getUppyUploadPlugin(
storageProvider,
fetch,
backendUrl
);
uppy2.use(plugin, options);
+ setIsUploading(true);
uppy2.addFile(_file);
+ uppy2.on('upload-error', (file, error) => {
+ setIsUploading(false);
+ toaster.show(`Failed to upload ${file.name}: ${error.message}`, 'error');
+ });
uppy2.on('complete', (result) => {
+ setIsUploading(false);
if (result) {
const mediaToAdd = result?.successful![0].response?.body;
if (mediaToAdd && mediaToAdd.path && mediaToAdd.id) {
const newMedia: {
path: string;
id: string;
}[] = [
...(value[index].image || []),
{ path: mediaToAdd.path, id: mediaToAdd.id },
];
changeImage(index)({ target: { name: 'image', value: newMedia } });
+ } else {
+ toaster.show('Invalid response from server', 'error');
}
}
});
};Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 413-413: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
There was a problem hiding this comment.
@caushcani code LGTM.
Please resolve conflicts for that PR.
|
@A1exKH solved. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/frontend/src/components/launches/add.edit.model.tsx (1)
573-600: Simplify paste handler implementationThe paste handler works correctly but could be more maintainable.
Consider these improvements:
-onPaste={(event) => { - const clipboardData = - event.clipboardData || - ( - window as Window & - typeof globalThis & { - clipboardData: DataTransfer; - } - ).clipboardData; - if (clipboardData && clipboardData.items) { - for (const item of Array.from(clipboardData.items)) { - const mediaTypes = ['image', 'video']; - - if (mediaTypes.some((type) => item.type.indexOf(type) !== -1)) { - const media = item.getAsFile(); - if(media){ - uploadMediaToServer(media, index); - } - } - } - } -}} +onPaste={(event) => { + const SUPPORTED_MEDIA_TYPES = ['image/', 'video/mp4']; + const items = event.clipboardData?.items ?? window.clipboardData?.items; + + if (!items) return; + + Array.from(items).forEach(item => { + if (SUPPORTED_MEDIA_TYPES.some(type => item.type.startsWith(type))) { + const media = item.getAsFile(); + if (media) { + uploadMediaToServer(media, index); + } + } + }); +}}🧰 Tools
🪛 Biome (1.9.4)
[error] 582-582: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/components/launches/add.edit.model.tsx(4 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
apps/frontend/src/components/launches/add.edit.model.tsx
[error] 436-436: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 582-582: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (2)
apps/frontend/src/components/launches/add.edit.model.tsx (2)
53-55: LGTM: Required imports added for media upload functionality.
The new imports provide the necessary dependencies for implementing the image paste feature.
414-448: 🛠️ Refactor suggestion
Add error handling and loading state management
While the upload implementation is functional, it needs improvements:
- Error handling for upload failures
- Loading state management
- Optional chaining for safer null handling
Consider applying these improvements:
const uploadMediaToServer = (_file: File, index: number) => {
+ const [isUploading, setIsUploading] = useState(false);
const uppy2 = new Uppy({
autoProceed: true,
restrictions: {
maxNumberOfFiles: 1,
allowedFileTypes: ['image/*', 'video/mp4'],
maxFileSize: 1000000000,
},
});
const { plugin, options } = getUppyUploadPlugin(
storageProvider,
fetch,
backendUrl
);
uppy2.use(plugin, options);
+ setIsUploading(true);
uppy2.addFile(_file);
+ uppy2.on('upload-error', (file, error) => {
+ setIsUploading(false);
+ toaster.show(`Failed to upload ${file.name}: ${error.message}`, 'error');
+ });
uppy2.on('complete', (result) => {
+ setIsUploading(false);
- if (result) {
- const mediaToAdd = result?.successful![0].response?.body;
+ const mediaToAdd = result?.successful?.[0]?.response?.body;
- if (mediaToAdd && mediaToAdd.path && mediaToAdd.id) {
+ if (mediaToAdd?.path && mediaToAdd?.id) {
const newMedia: {
path: string;
id: string;
}[] = [
...(value[index].image || []),
{ path: mediaToAdd.path, id: mediaToAdd.id },
];
changeImage(index)({ target: { name: 'image', value: newMedia } });
+ } else {
+ toaster.show('Invalid response from server', 'error');
}
- }
});
};Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 436-436: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
What kind of change does this PR introduce?
Feature: Enable pasting of pictures into the editor, with automatic upload and attachment.
Why was this change needed?
This feature was needed to streamline the process of adding images and videos to the editor. Currently, users must go through a more time-consuming method of uploading media, and this change aims to make the process faster by allowing images/videos to be pasted directly into the editor, automatically uploading them as media and attaching them.
Other information:
eg: Did you discuss this change with anybody before working on it (not required, but can be a good idea for bigger changes). Any plans for the future, etc?
Issue #390 and i discussed it with @nevo-david .
Checklist:
Put a "X" in the boxes below to indicate you have followed the checklist;
Summary by CodeRabbit
New Features
Bug Fixes
Documentation